Skip to content
This repository has been archived by the owner on Sep 7, 2020. It is now read-only.

Fix coverage report on CI #296

Merged
merged 1 commit into from
Mar 12, 2016
Merged

Fix coverage report on CI #296

merged 1 commit into from
Mar 12, 2016

Conversation

thangngoc89
Copy link
Contributor

It's good to have coverage report back.

Right @codecov-io ?

@codecov-io
Copy link

Current coverage is 68.28%

Merging #296 into next will increase coverage by +6.76% as of 500c9dc

@@             next    #296   diff @@
=====================================
  Files          40      42     +2
  Stmts         525     637   +112
  Branches        0       0       
  Methods                         
=====================================
+ Hit           323     435   +112
  Partial         0       0       
  Missed        202     202       

Review entire Coverage Diff as of 500c9dc

Powered by Codecov. Updated on successful CI builds.

@MoOx
Copy link
Owner

MoOx commented Mar 12, 2016

why is this weird change required?

@thangngoc89
Copy link
Contributor Author

It's AVA who spawn those processes, not npm run. nyc 6.0.0 isn't required

Hang on a sec. nyc doesn't ignore test files

@MoOx
Copy link
Owner

MoOx commented Mar 12, 2016

It was working before right?

@thangngoc89
Copy link
Contributor Author

@MoOx yes. It was working before. No idea why it's stop working

@thangngoc89
Copy link
Contributor Author

Related ? istanbuljs/nyc#190

@arrowrowe
Copy link

@thangngoc89 I cannot understand what causes #190. The problem is, in short, no coverage files found only using Node 5.8.0 and NPM 3.7.3. This problem disappears after updating NPM to 3.8.1 or using Node 5.7.1 instead.

@thangngoc89
Copy link
Contributor Author

@arrowrowe We are having the same problem here. node 4 is fine. Pushed a new commit to use latest npm

@arrowrowe
Copy link

@thangngoc89 Thanks! Trying it now.

@thangngoc89
Copy link
Contributor Author

@MoOx upgrading npm to latest version (3.8.1) did help

https://travis-ci.org/MoOx/statinamic/builds/115487336#L654-L728

@arrowrowe
Copy link

@thangngoc89 It works for me as well. Thanks!

https://travis-ci.org/arrowrowe/romi/jobs/115487555

@thangngoc89 thangngoc89 changed the title Fix coverage report on CI. Upgrade nyc to 6.0 Fix coverage report on CI Mar 12, 2016
@MoOx
Copy link
Owner

MoOx commented Mar 12, 2016

So no need for this PR right?

@thangngoc89
Copy link
Contributor Author

@MoOx We can wait until

  • nyc fixes this bug
  • or a new node release which uses npm@3.8.1

@MoOx
Copy link
Owner

MoOx commented Mar 12, 2016

latest node don't get npm@^3.0.0 ? They fix the version to 3.7.x ?

@thangngoc89
Copy link
Contributor Author

@MoOx
Copy link
Owner

MoOx commented Mar 12, 2016

npm*

But does latest node ship this version? It seems latest node ship 3.7.x https://github.com/nodejs/node/tree/master/deps/npm (or https://github.com/nodejs/node/tree/v5.x/deps/npm )

@MoOx
Copy link
Owner

MoOx commented Mar 12, 2016

Oh my bad. I just looked at the nyc mentionned issue. So not sure what to do.

@MoOx
Copy link
Owner

MoOx commented Mar 12, 2016

We get your patch for now. Can you add a comment in it ?

@thangngoc89
Copy link
Contributor Author

Added comment

MoOx added a commit that referenced this pull request Mar 12, 2016
Fix coverage report on CI (via workaround)
@MoOx MoOx merged commit df06145 into next Mar 12, 2016
@MoOx MoOx deleted the fix/coverage branch March 12, 2016 08:30
@MoOx
Copy link
Owner

MoOx commented Mar 12, 2016

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants